add umfPoolTrimMemory #1318
add umfPoolTrimMemory #1318
Conversation
bratpiorka
force-pushed
the
rrudnick_dp_trim
branch
from
6de8410 to
6a3ae03
Compare
bratpiorka
force-pushed
the
rrudnick_dp_trim
branch
3 times, most recently
from
4a3b48f to
4dee8a2
Compare
bratpiorka
force-pushed
the
rrudnick_dp_trim
branch
2 times, most recently
from
a699662 to
c151a5c
Compare
src/pool/pool_disjoint.c
Outdated
| if (hPool == NULL) { | ||
| return UMF_RESULT_ERROR_INVALID_ARGUMENT; | ||
| } |
There was a problem hiding this comment.
If you introduce UMF_CHECK above, please apply it here as well.
There was a problem hiding this comment.
changed to assert as this is checked in the upper layer
test/memoryPoolAPI.cpp
Outdated
| // TODO enable jemalloc pool tests | ||
| //poolCreateExtParams{umfJemallocPoolOps(), nullptr, nullptr, | ||
| // &BA_GLOBAL_PROVIDER_OPS, nullptr, nullptr})); |
| ops->ext_trim_memory(pool, pool->buckets[0]->size); | ||
| EXPECT_EQ(pool->buckets[0]->available_slabs_num, 1); | ||
| EXPECT_EQ(pool->buckets[0]->curr_slabs_in_pool, 1); | ||
|
|
There was a problem hiding this comment.
perhaps add testing various values of minBytesToKeep
There was a problem hiding this comment.
added new case with 3 * slab_size to keep
There was a problem hiding this comment.
thx, but now the CI is failing...? ;)
bratpiorka
force-pushed
the
rrudnick_dp_trim
branch
2 times, most recently
from
6d877f9 to
c523215
Compare
bratpiorka
force-pushed
the
rrudnick_dp_trim
branch
7 times, most recently
from
02c6e96 to
c2a5994
Compare
src/pool/pool_disjoint.c
Outdated
| disjoint_pool_t *hPool = (disjoint_pool_t *)pool; | ||
|
|
||
| // tracking the number of bytes left to keep | ||
| int bytesLeftToKeep = (int)minBytesToKeep; |
There was a problem hiding this comment.
You cast from 8 byte unsigned variable to 4 byte signed variable
src/pool/pool_scalable.c
Outdated
| (void)pool; // unused | ||
| (void)minBytesToKeep; // unused | ||
|
|
||
| return UMF_RESULT_SUCCESS; |
There was a problem hiding this comment.
Technically it is success, as this function has no promises, but for me it is ERROR NONSUPPORTED
There was a problem hiding this comment.
ok, removed - the umfDefaultTrimMemory() will be called here which returns UMF_RESULT_ERROR_NOT_SUPPORTED
| // use CTL to get the current memory pool size (if supported) | ||
| size_t reserved_memory1 = 0; | ||
| ret = umfCtlGet("umf.pool.by_handle.{}.stats.reserved_memory", | ||
| &reserved_memory1, sizeof(size_t), hPool); | ||
| if (ret == UMF_RESULT_SUCCESS) { | ||
| ASSERT_GE(reserved_memory1, 0ull); | ||
| } |
There was a problem hiding this comment.
It supported by all pools now
There was a problem hiding this comment.
I left the check here as this is the current state - I do not want to deal with backward compat issues in the future
test/poolFixtures.hpp
Outdated
| ret = umfCtlGet("umf.pool.by_handle.{}.stats.reserved_memory", | ||
| &reserved_memory2, sizeof(size_t), hPool); | ||
| if (ret == UMF_RESULT_SUCCESS) { | ||
| ASSERT_EQ(reserved_memory2, 0ull); |
There was a problem hiding this comment.
Do this test work with tbbpool - it does nothing on trim so this test should fail. Why it is not failing?
bratpiorka
force-pushed
the
rrudnick_dp_trim
branch
from
c2a5994 to
6d5b0c0
Compare
|
@KFilipek @lukaszstolarczuk @lplewa please re-review |
implement umfPoolTrimMemory